Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Jul 14, 2025

No description provided.

Copilot AI review requested due to automatic review settings July 14, 2025 14:14
@lerouxb lerouxb requested a review from a team as a code owner July 14, 2025 14:14
@github-actions github-actions bot added the fix label Jul 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new CLI flag --versions to print the full process.versions JSON (Node, Electron, Chromium) and updates the end-to-end test setup to dynamically derive the Chromium version from the target Electron binary (packaged or monorepo) to avoid mismatches.

  • Add --versions flag in the Compass main process to output all process versions.
  • Extend preferences schema to include a versions boolean flag.
  • Rename test-runner constants to MONOREPO_ELECTRON_* and implement getChromiumVersionFromBinary for packaged-app tests.
  • Update E2E helpers (compass.ts and compass-web-sandbox.ts) to use the correct Chromium version depending on context.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/compass/src/main/index.ts Added --versions CLI check to dump process.versions.
packages/compass-preferences-model/src/preferences-schema.tsx Added versions preference and schema entry for the new flag.
packages/compass-e2e-tests/helpers/test-runner-paths.ts Renamed Electron version exports to distinguish monorepo.
packages/compass-e2e-tests/helpers/compass.ts Imported new monorepo constant, added getChromiumVersionFromBinary, and conditional version logic.
packages/compass-e2e-tests/helpers/compass-web-sandbox.ts Switched to use MONOREPO_ELECTRON_CHROMIUM_VERSION import.
Comments suppressed due to low confidence (2)

packages/compass/src/main/index.ts:60

  • The new --versions flag isn't mentioned in the CLI help output; consider updating getHelpText() or the usage instructions to document this option.
  if (preferences.versions) {

packages/compass-e2e-tests/helpers/compass.ts:478

  • The new getChromiumVersionFromBinary function isn’t covered by any tests; adding unit or integration tests would ensure it correctly parses and converts the Electron version.
async function getChromiumVersionFromBinary(path: string) {

Comment on lines +478 to +479
async function getChromiumVersionFromBinary(path: string) {
const { stdout } = await execFileIgnoreError(path, ['--versions'], {});
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The parameter name path shadows the imported path module; consider renaming it (e.g., to binaryPath) to avoid confusion.

Suggested change
async function getChromiumVersionFromBinary(path: string) {
const { stdout } = await execFileIgnoreError(path, ['--versions'], {});
async function getChromiumVersionFromBinary(binaryPath: string) {
const { stdout } = await execFileIgnoreError(binaryPath, ['--versions'], {});

Copilot uses AI. Check for mistakes.
@lerouxb lerouxb added the no release notes Fix or feature not for release notes label Jul 14, 2025
Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question / suggestion (that I don't think is that important), otherwise looks good to me, hopefully will help with the smokes!

@lerouxb lerouxb changed the title fix: pull the electron version from the binary, not the monorepo when testing the packaged app COMPASS-9421 fix: pull the chromium version from the binary, not the monorepo when testing the packaged app COMPASS-9421 Jul 14, 2025
@lerouxb lerouxb merged commit 7864673 into main Jul 14, 2025
45 of 56 checks passed
@lerouxb lerouxb deleted the versions-flag branch July 14, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants